feat(clerk-expo): Add native Apple Sign-In support for iOS #7053
Conversation
- Introduced `useAppleSignIn` hook for native Apple Sign-In on iOS using expo-apple-authentication. - Implemented flow to handle sign-in and sign-up using Clerk's backend. - Added type definitions for parameters and return types of the hook. - Updated `pnpm-lock.yaml` to include expo-apple-authentication as a dependency. - Modified strategies.ts to include 'oauth_token_apple' strategy type.
…orresponding tests
🦋 Changeset detectedLatest commit: c4287a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe PR introduces native Apple Sign-In support for iOS through a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Hook as useSignInWithApple
participant Clerk as Clerk Auth
participant Apple as Apple Auth
participant Crypto as Crypto Module
User->>Hook: Call startAppleAuthenticationFlow()
Hook->>Hook: Check Clerk loaded
Hook->>Apple: Check isAvailableAsync()
alt Apple Auth Available
Hook->>Crypto: Generate nonce (randomUUID)
Hook->>Apple: requestAsync(nonce, email, fullName)
Apple-->>Hook: identityToken + credentials
Hook->>Clerk: signIn.create(oauth_token_apple, token)
alt Existing User
Clerk-->>Hook: createdSessionId
Hook->>Clerk: setActive(session)
Hook-->>User: {createdSessionId, setActive, signIn}
else New User (Transferable)
Hook->>Clerk: signUp.create(transfer, metadata)
Clerk-->>Hook: createdSessionId
Hook->>Clerk: setActive(session)
Hook-->>User: {createdSessionId, setActive, signUp}
end
else Unavailable or Error
Hook-->>User: Error thrown
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/expo/src/hooks/useAppleSignIn.ts (2)
86-87: Consider using cryptographically secure nonce generation.The current nonce generation using
Math.random()is not cryptographically secure. While this may be acceptable if Clerk's backend validates the identity token independently, consider using a more secure random source.Consider using expo-crypto for secure random generation:
+import * as Crypto from 'expo-crypto'; + // Generate a nonce for the Apple Sign-In request (required by Clerk) -const nonce = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15); +const nonce = Crypto.randomUUID();Note: This would require adding expo-crypto as a dependency. Verify with the Clerk backend team whether the current approach meets security requirements.
105-109: Consider removing type assertions once types are fully propagated.The
as anycasts on lines 107 and 109 bypass TypeScript's type checking. While this is acceptable during the rollout of new types, consider:
- Add a comment explaining why the casts are temporarily needed:
// Create a SignIn with the Apple ID token strategy +// Note: Type assertions needed until @clerk/clerk-react types are updated await signIn.create({ strategy: 'oauth_token_apple' as any, token: identityToken, } as any);
- Alternatively, if the types in
@clerk/typesare now properly defined, verify whether these casts can be removed entirely by ensuring@clerk/clerk-reacthas the updated types.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/brave-apples-sign.md(1 hunks)packages/expo/package.json(3 hunks)packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts(1 hunks)packages/expo/src/hooks/index.ts(1 hunks)packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)packages/types/src/signInCommon.ts(2 hunks)packages/types/src/signUpCommon.ts(2 hunks)packages/types/src/strategies.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/expo/package.jsonpackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/expo/src/hooks/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/expo/src/hooks/index.ts
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/expo/package.json
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/brave-apples-sign.md
🧬 Code graph analysis (4)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
useAppleSignIn(56-156)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
packages/types/src/signUpCommon.ts (1)
packages/types/src/strategies.ts (1)
AppleIdTokenStrategy(5-5)
packages/types/src/signInCommon.ts (1)
packages/types/src/strategies.ts (1)
AppleIdTokenStrategy(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (21)
packages/expo/package.json (1)
97-97: LGTM! Dependency configuration follows established patterns.The expo-apple-authentication package is correctly configured across devDependencies, peerDependencies, and peerDependenciesMeta, following the same pattern as other optional Expo packages like expo-local-authentication and expo-secure-store.
Also applies to: 106-106, 119-121
.changeset/brave-apples-sign.md (1)
1-6: LGTM! Changeset properly documents the new feature.The version bumps are appropriate: minor for @clerk/clerk-expo (new feature) and patch for @clerk/types (type additions). The description clearly states the feature and requirements.
packages/types/src/signUpCommon.ts (1)
12-12: LGTM! Type extension properly supports Apple ID token-based signup.The AppleIdTokenStrategy is correctly imported and added to the SignUpCreateParams strategy union, enabling the new Apple Sign-In flow during user registration.
Also applies to: 93-93
packages/expo/src/hooks/index.ts (1)
14-14: LGTM! Export follows established barrel pattern.The useAppleSignIn hook is correctly exported alongside other Expo-specific hooks.
packages/types/src/strategies.ts (1)
5-5: LGTM! Strategy type follows naming conventions.The AppleIdTokenStrategy type is correctly defined with the 'oauth_token_apple' literal, following the established pattern for token-based strategies.
packages/types/src/signInCommon.ts (1)
43-43: LGTM! Type extension correctly supports Apple ID token-based sign-in.The AppleIdTokenStrategy is properly imported and added to SignInCreateParams as a discriminated union variant with the required token field, following the same pattern as GoogleOneTapStrategy.
Also applies to: 141-144
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (6)
6-39: LGTM! Test mocks are properly configured.The test setup correctly mocks all dependencies (@clerk/clerk-react, expo-apple-authentication, react-native) using vitest's hoisting pattern, ensuring isolated and reliable tests.
41-76: LGTM! Test fixtures and lifecycle hooks are well-structured.The mock objects correctly represent Clerk's SignIn and SignUp resources, and the beforeEach/afterEach hooks properly initialize and clean up the test state.
79-84: LGTM! Basic hook structure test is appropriate.The test verifies the hook returns the expected API surface.
86-157: LGTM! Sign-in flow tests comprehensively cover main scenarios.The tests effectively verify:
- Successful sign-in for existing users with proper token exchange
- Transfer flow for new users requiring sign-up
- Graceful handling of user cancellation
All assertions validate the correct behavior and return values.
159-179: LGTM! Error handling tests validate proper error conditions.The tests correctly verify that appropriate errors are thrown when:
- Apple Authentication is unavailable on the device
- No identity token is received from Apple
181-195: LGTM! Early return test validates defensive loading check.The test correctly verifies that when Clerk isn't fully loaded, the hook returns early without attempting Apple Authentication, preventing potential runtime errors.
packages/expo/src/hooks/useAppleSignIn.ts (9)
19-55: LGTM! Excellent documentation with practical example.The JSDoc provides comprehensive documentation including a realistic usage example, platform requirements, and peer dependency information.
56-58: LGTM! Hook initialization correctly uses Clerk hooks.Both useSignIn and useSignUp are properly called to obtain the necessary resources and loading states.
64-68: LGTM! Platform check with helpful error message.The iOS-only restriction is correctly enforced with a clear error message that guides developers to use the alternative OAuth flow on other platforms.
70-77: LGTM! Proper defensive check for Clerk loading state.The early return when Clerk isn't loaded prevents potential runtime errors and returns a consistent structure for callers to handle.
79-83: LGTM! Device capability check is appropriate.The availability check ensures Apple Authentication is supported before proceeding, with a clear error message for unsupported devices.
98-103: LGTM! Identity token validation is appropriate.The code correctly validates that an identity token was received from Apple before proceeding.
111-127: LGTM! Transfer flow correctly handles new user sign-up.The code properly detects when a user doesn't exist (transferable status) and creates a SignUp with the transfer flag, preserving any unsafe metadata provided by the caller.
129-150: LGTM! Error handling gracefully manages cancellation and propagates other errors.The code correctly:
- Returns the sign-in session ID for existing users
- Handles user cancellation gracefully by returning null without throwing
- Propagates other errors for proper error handling upstream
153-155: LGTM! Hook return follows React conventions.The hook cleanly returns an object with the startAppleSignInFlow function.
What!? I will fix this... I don't think thats correct. |
Ahhh... This is everyone elses changes since my branch was created. Only two of my packages will be updated. |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.ts
🧬 Code graph analysis (1)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/expo/src/hooks/useAppleSignIn.ts (7)
2-2: Past review issue resolved.The missing
SignUpUnsafeMetadataimport flagged in the previous review has been correctly added.
19-55: Excellent documentation.The JSDoc is comprehensive, includes a practical example, and clearly documents platform restrictions and peer dependencies.
64-68: Good platform enforcement.The iOS-only check with a descriptive error message guiding users to the web OAuth alternative is well-implemented.
113-129: Transfer flow logic is well-implemented.The conditional handling of the
transferablestatus correctly routes new users through the SignUp flow while preserving optional metadata.
138-152: Robust error handling.The distinction between user cancellation (
ERR_REQUEST_CANCELED) and other errors is appropriate. Returning gracefully on cancellation allows the UI to handle the flow without treating it as an error.
155-157: Clean hook interface.The return structure follows React hook conventions and provides a clear API surface.
108-111: The review comment is based on flawed assumptions about type availability.The
AppleIdTokenStrategytype does exist in@clerk/types, but it's not included in thestrategyparameter union ofSignInFutureCreateParams. Thestrategyfield acceptsOAuthStrategy | 'saml' | 'enterprise_sso' | PasskeyStrategy, andAppleIdTokenStrategy('oauth_token_apple') is not part of this union.Using
as AppleIdTokenStrategy(as suggested) would not eliminate the type assertion problem—TypeScript would still reject it sinceAppleIdTokenStrategyis not an accepted strategy type. The currentas anyapproach is a reasonable temporary workaround until the type definition inSignInFutureCreateParamsis updated to includeAppleIdTokenStrategyin its union.Likely an incorrect or invalid review comment.
chriscanin
changed the title
chriscanin
changed the title
chriscanin
changed the title
…eneration in useAppleSignIn hook - Added expo-crypto as a dependency in package.json. - Updated useAppleSignIn hook to use Crypto.randomUUID() for generating a cryptographically secure nonce instead of Math.random(). - Mocked expo-crypto in useAppleSignIn tests to return a fixed UUID for consistent testing. - Updated tests to verify that randomUUID is called and the correct nonce is used in the sign-in flow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
138-152: Consider usingunknownfor caught errors.The error handling logic is correct, but the error parameter uses
anywhich goes against TypeScript best practices.Apply this diff to improve type safety:
- } catch (error: any) { + } catch (error: unknown) { // Handle Apple Authentication errors - if (error?.code === 'ERR_REQUEST_CANCELED') { + if (error && typeof error === 'object' && 'code' in error && error.code === 'ERR_REQUEST_CANCELED') { // User canceled the sign-in flowAlternatively, define a type guard:
function isAppleAuthError(error: unknown): error is { code: string } { return typeof error === 'object' && error !== null && 'code' in error; } // Then use: if (isAppleAuthError(error) && error.code === 'ERR_REQUEST_CANCELED') {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/expo/package.json(2 hunks)packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts(1 hunks)packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/expo/package.json
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
🧬 Code graph analysis (2)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
useAppleSignIn(57-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (8)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (3)
1-46: LGTM! Well-structured test setup.The mock setup is comprehensive and properly uses
vi.hoisted()for hoisting. All dependencies are appropriately mocked with realistic API surfaces.
48-84: LGTM! Proper test lifecycle management.The test fixtures and lifecycle hooks are well-organized. Mocks are properly reset before each test and restored after, ensuring test isolation.
86-205: LGTM! Comprehensive test coverage.The test suite thoroughly covers all critical paths:
- ✅ Happy path for existing users with session creation
- ✅ Transfer flow for new user sign-up
- ✅ Graceful handling of user cancellation
- ✅ Error cases (unavailable authentication, missing token)
- ✅ Early return when Clerk is not loaded
All assertions properly verify function calls, parameters, and return values.
packages/expo/src/hooks/useAppleSignIn.ts (5)
1-18: LGTM! Clean imports and type definitions.The type definitions are well-structured and properly exported. The import of
SignUpUnsafeMetadatafrom a previous review has been correctly addressed.
20-56: LGTM! Excellent documentation.The JSDoc is comprehensive and follows best practices. The example is clear and demonstrates the complete flow including session activation.
57-78: LGTM! Proper hook initialization and guards.The hook correctly uses Clerk hooks at the top level and implements appropriate guards for platform compatibility and loaded state. The error message for non-iOS platforms helpfully suggests an alternative approach.
80-137: LGTM! Authentication flow correctly implemented.The flow properly handles:
- Device capability checking
- Cryptographically secure nonce generation (addressed from previous review)
- Identity token validation
- Transfer flow for new users vs. existing user sign-in
The
as anytype assertions on lines 109 and 111 are acceptable given the inline comment explaining they're temporary until the newoauth_token_applestrategy type propagates through the type system.
155-158: LGTM! Clean hook interface.The hook properly returns an object with the
startAppleSignInFlowfunction, following React hook conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/expo/src/hooks/useSignInWithApple.ios.ts (2)
98-129: Dropanycasts and use returned resources to avoid stale reads.Remove the
as anyand rely on updated types. Also use the return value from signIn.create (and signUp.create) instead of reading the pre-callsignInobject.As per coding guidelines.
- // Create a SignIn with the Apple ID token strategy - // Note: Type assertions needed until @clerk/clerk-react propagates the new oauth_token_apple strategy type - await signIn.create({ - strategy: 'oauth_token_apple' as any, - token: identityToken, - } as any); - - // Check if we need to transfer to SignUp (user doesn't exist yet) - const userNeedsToBeCreated = signIn.firstFactorVerification.status === 'transferable'; + // Create a SignIn with the Apple ID token strategy + const updatedSignIn = await signIn.create({ + strategy: 'oauth_token_apple', + token: identityToken, + }); + + // Check if we need to transfer to SignUp (user doesn't exist yet) + const userNeedsToBeCreated = updatedSignIn.firstFactorVerification.status === 'transferable'; @@ - await signUp.create({ + const updatedSignUp = await signUp.create({ transfer: true, unsafeMetadata: startAppleAuthenticationFlowParams?.unsafeMetadata, }); return { - createdSessionId: signUp.createdSessionId, + createdSessionId: updatedSignUp.createdSessionId, setActive, - signIn, - signUp, + signIn: updatedSignIn, + signUp: updatedSignUp, }; } // User exists - return the SignIn session return { - createdSessionId: signIn.createdSessionId, + createdSessionId: updatedSignIn.createdSessionId, setActive, - signIn, + signIn: updatedSignIn, signUp, };
1-10: Import missing SignUpUnsafeMetadata type.The params type uses SignUpUnsafeMetadata but it’s not imported; TS will fail.
As per coding guidelines.
-import type { SetActive, SignInResource, SignUpResource } from '@clerk/types'; +import type { SetActive, SignInResource, SignUpResource, SignUpUnsafeMetadata } from '@clerk/types';packages/expo/src/hooks/useSignInWithApple.ts (1)
1-7: Import missing SignUpUnsafeMetadata type.StartAppleAuthenticationFlowParams references SignUpUnsafeMetadata but it’s not imported; this breaks type-checking. Add it to the type-only import.
As per coding guidelines.
-import type { SetActive, SignInResource, SignUpResource } from '@clerk/types'; +import type { SetActive, SignInResource, SignUpResource, SignUpUnsafeMetadata } from '@clerk/types';packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts (1)
142-147: Fix misnamed hook and method in tests.Rename to the actual exported names: useSignInWithApple and startAppleAuthenticationFlow.
- const { result } = renderHook(() => useAppleSignIn()); + const { result } = renderHook(() => useSignInWithApple()); @@ - const response = await result.current.startAppleSignInFlow({ + const response = await result.current.startAppleAuthenticationFlow({ unsafeMetadata: { source: 'test' }, });- const { result } = renderHook(() => useAppleSignIn()); - await expect(result.current.startAppleSignInFlow()).rejects.toThrow( + const { result } = renderHook(() => useSignInWithApple()); + await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow( 'Apple Authentication is not available on this device.', );- const { result } = renderHook(() => useAppleSignIn()); - await expect(result.current.startAppleSignInFlow()).rejects.toThrow( + const { result } = renderHook(() => useSignInWithApple()); + await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow( 'No identity token received from Apple Sign-In.', );Also applies to: 174-179, 186-191
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts(1 hunks)packages/expo/src/hooks/useSignInWithApple.ios.ts(1 hunks)packages/expo/src/hooks/useSignInWithApple.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useSignInWithApple.tspackages/expo/src/hooks/__tests__/useSignInWithApple.test.tspackages/expo/src/hooks/useSignInWithApple.ios.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
🧬 Code graph analysis (3)
packages/expo/src/hooks/useSignInWithApple.ts (4)
packages/expo/src/hooks/useSignInWithApple.ios.ts (3)
StartAppleAuthenticationFlowParams(8-10)StartAppleAuthenticationFlowReturnType(12-17)useSignInWithApple(56-150)packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts (2)
packages/expo/src/hooks/useSignInWithApple.ios.ts (1)
useSignInWithApple(56-150)packages/expo/src/hooks/useSignInWithApple.ts (1)
useSignInWithApple(52-69)
packages/expo/src/hooks/useSignInWithApple.ios.ts (4)
packages/expo/src/hooks/useSignInWithApple.ts (3)
StartAppleAuthenticationFlowParams(5-7)StartAppleAuthenticationFlowReturnType(9-14)useSignInWithApple(52-69)packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
…s for consistency
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts (8)
23-32: Harden mock shape for expo-apple-authentication.Support both star and default import patterns to avoid brittle module resolution issues.
Apply:
-vi.mock('expo-apple-authentication', () => { - return { - signInAsync: mocks.signInAsync, - isAvailableAsync: mocks.isAvailableAsync, - AppleAuthenticationScope: { - FULL_NAME: 0, - EMAIL: 1, - }, - }; -}); +vi.mock('expo-apple-authentication', () => { + const AppleAuthenticationScope = { FULL_NAME: 0, EMAIL: 1 }; + const mod = { + signInAsync: mocks.signInAsync, + isAvailableAsync: mocks.isAvailableAsync, + AppleAuthenticationScope, + }; + return { __esModule: true, ...mod, default: mod }; +});
52-83: Avoid cross-test state leakage; re-initialize per test.mockSignIn.firstFactorVerification.status is mutated across tests. Re-create fresh objects in beforeEach to remove order coupling.
Apply:
- const mockSignIn = { - create: vi.fn(), - createdSessionId: 'test-session-id', - firstFactorVerification: { - status: 'verified', - }, - }; - - const mockSignUp = { - create: vi.fn(), - createdSessionId: null, - }; - - const mockSetActive = vi.fn(); + let mockSignIn: { + create: ReturnType<typeof vi.fn>; + createdSessionId: string | null; + firstFactorVerification: { status: 'verified' | 'transferable' }; + }; + let mockSignUp: { create: ReturnType<typeof vi.fn>; createdSessionId: string | null }; + let mockSetActive: ReturnType<typeof vi.fn>; @@ - beforeEach(() => { - vi.clearAllMocks(); - - mocks.useSignIn.mockReturnValue({ - signIn: mockSignIn, - setActive: mockSetActive, - isLoaded: true, - }); - - mocks.useSignUp.mockReturnValue({ - signUp: mockSignUp, - isLoaded: true, - }); + beforeEach(() => { + vi.clearAllMocks(); + + mockSignIn = { + create: vi.fn(), + createdSessionId: 'test-session-id', + firstFactorVerification: { status: 'verified' }, + }; + mockSignUp = { + create: vi.fn(), + createdSessionId: null, + }; + mockSetActive = vi.fn(); + + mocks.useSignIn.mockReturnValue({ signIn: mockSignIn, setActive: mockSetActive, isLoaded: true }); + mocks.useSignUp.mockReturnValue({ signUp: mockSignUp, isLoaded: true });
85-88: Prefer clear/reset over restore for hoisted module mocks.restoreAllMocks mainly affects spies; with hoisted vi.mock, prefer clearAllMocks (already in beforeEach) or resetAllMocks in afterEach to avoid accidentally restoring implementations.
Apply one of:
- afterEach(() => { - vi.restoreAllMocks(); - }); + afterEach(() => { + vi.clearAllMocks(); + });or
- afterEach(() => { - vi.restoreAllMocks(); - }); + afterEach(() => { + vi.resetAllMocks(); + });
97-125: Strengthen assertions for Apple scopes and call shape.Verify requested scopes exactly match FULL_NAME and EMAIL to catch regressions.
Apply:
expect(mocks.signInAsync).toHaveBeenCalledWith( expect.objectContaining({ - requestedScopes: expect.any(Array), + requestedScopes: expect.any(Array), nonce: 'test-nonce-uuid', }), ); + const callArg = mocks.signInAsync.mock.calls[0][0]; + expect(callArg.requestedScopes).toEqual([0, 1]); // FULL_NAME, EMAIL
127-157: Also assert setActive is forwarded in transfer flow.Confirms parity with existing-user path.
Apply:
expect(response.createdSessionId).toBe('new-user-session-id'); + expect(response.setActive).toBe(mockSetActive);
171-179: Assert no downstream calls when Apple is unavailable.Ensure we short-circuit before attempting sign-in or generating nonce.
Apply:
await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow( 'Apple Authentication is not available on this device.', ); + expect(mocks.randomUUID).not.toHaveBeenCalled(); + expect(mocks.signInAsync).not.toHaveBeenCalled();
181-191: Assert no signIn.create when identity token is missing.Guards against accidental backend calls without a token.
Apply:
await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow( 'No identity token received from Apple Sign-In.', ); + expect(mockSignIn.create).not.toHaveBeenCalled();
193-207: Add coverage for signUp not loaded and assert nonce/signIn are skipped.Completes early-return matrix and tightens guarantees.
Apply:
test('should return early when clerk is not loaded', async () => { @@ expect(mocks.signInAsync).not.toHaveBeenCalled(); expect(response.createdSessionId).toBe(null); }); + + test('should return early when signUp is not loaded', async () => { + mocks.useSignUp.mockReturnValue({ + signUp: mockSignUp, + isLoaded: false, + }); + + const { result } = renderHook(() => useSignInWithApple()); + const response = await result.current.startAppleAuthenticationFlow(); + + expect(mocks.isAvailableAsync).not.toHaveBeenCalled(); + expect(mocks.randomUUID).not.toHaveBeenCalled(); + expect(mocks.signInAsync).not.toHaveBeenCalled(); + expect(response.createdSessionId).toBe(null); + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts
🧬 Code graph analysis (1)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts (1)
packages/expo/src/hooks/useSignInWithApple.ios.ts (1)
useSignInWithApple(56-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts (2)
34-41: LGTM on expo-crypto mocking.Covering both default and named exports avoids import-mode pitfalls.
16-21: Import source verified—mocks will apply correctly.The implementation imports
useSignInanduseSignUpfrom@clerk/clerk-react(line 1 of useSignInWithApple.ios.ts), which matches the test mock target. The test mocks are correctly configured and will apply as intended.
|
Hi @dstaley & @mikepitre , this PR is ready for review, and the docs and quickstart repo have been updated as well: PS: I realize now with the github enterprise "draft" commits can be made, I will make sure to draft this next time, to not spam inbox and such! |
…/mobile-279-expo-sign-in-with-apple
This comment has been minimized.
This comment has been minimized.
…r SDK consistency
…uests for SDK consistency" This reverts commit 9a6aa39.
dstaley
left a comment
dstaley
left a comment
There was a problem hiding this comment.
two minor non-blocking refactoring comments, but otherwise lgtm pending a decision on whether the API needs to change to align with the new custom flow APIs landing in the upcoming major
|
|
||
| import { errorThrower } from '../utils/errors'; | ||
|
|
||
| export type StartAppleAuthenticationFlowParams = { |
There was a problem hiding this comment.
would it make sense to move these to a types file and then export * from './useSignInWithApple.types.ts? I'm not sure how TypeScript specifically works in instances where we're using platform-specific files, but it would eliminate having the types duplicated.
| function startAppleAuthenticationFlow( | ||
| _startAppleAuthenticationFlowParams?: StartAppleAuthenticationFlowParams, | ||
| ): Promise<StartAppleAuthenticationFlowReturnType> { | ||
| return errorThrower.throw( |
There was a problem hiding this comment.
should this throw an error on calls to useSignInWithApple instead of startAppleAuthenticationFlow? That way it throws the error on render rather than on user interaction, which a dev might miss.
Description
useAppleSignInhook for native Apple Sign-In on iOS using expo-apple-authentication.Fixes: https://linear.app/clerk/project/native-apple-login-flow-in-expo-f94b4842afd2
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Docs PR: Chris/mobile 279 expo sign in with apple clerk-docs#2732
Expo Quickstart PR: Chris/mobile 279 expo sign in with apple clerk-expo-quickstart#15
Type of change
Summary by CodeRabbit
Release Notes
New Features
Chores